Revert #536 "perf: remove context threading in various pointer abstractions"#611
Conversation
NVIDIA#536)" This reverts commit 9a56516. This changed the public API of `MemoryPointer` and related classes, and the context that they held was used by Arrow (see apache/arrow#48259 (comment)): > Numba interop tests fail with: ``` arrow-dev/lib/python3.12/site-packages/pyarrow/tests/test_cuda_numba_interop.py:233: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ > ??? E TypeError: MemoryPointer.__init__() got multiple values for argument 'pointer' ``` This commit reverts the change, as it was intended to improve performance without changing functionality, but has had a functional change as a side effect. Following the merge of this PR, we should be able to remove some of the `@require_context` decorators with some more targeted changes.
|
Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
/ok to test |
Greptile OverviewGreptile SummaryThis PR cleanly reverts PR #536, which removed the Key Changes:
Justification: Confidence Score: 5/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User as External User/Arrow
participant API as from_cuda_array_interface()
participant Context as CUDA Context
participant MemPtr as MemoryPointer
participant DevArray as DeviceNDArray
Note over User,DevArray: Before Revert (PR #536 - BROKEN)
User->>API: Call with cuda-array-interface
API->>MemPtr: MemoryPointer(pointer, size, owner)
Note over MemPtr: No context parameter!
MemPtr-->>API: Memory pointer instance
API->>DevArray: Create DeviceNDArray
DevArray-->>User: Return array
Note over User: ❌ Arrow breaks: multiple values for 'pointer'
Note over User,DevArray: After Revert (Current PR - FIXED)
User->>API: Call with cuda-array-interface
API->>Context: current_context()
Context-->>API: Context instance
API->>MemPtr: MemoryPointer(context, pointer, size, owner)
Note over MemPtr: Context restored as first param
MemPtr->>MemPtr: Store context reference
MemPtr-->>API: Memory pointer instance
API->>DevArray: Create DeviceNDArray
DevArray-->>User: Return array
Note over User: ✅ Arrow works correctly
|
|
Following this PR: create an issue to track the work of re-removing |
- Revert NVIDIA#536 "perf: remove context threading in various pointer abstractions" (NVIDIA#611) - fix: empty array type mismatch between host and device (NVIDIA#612) - fix: warp vote operations must use a constant int for the `mode` parameter (NVIDIA#606)
### What changes are included in this PR? 1. Add compatibility fix for CUDA 13 in C++ CUDA tests 2. Add CI builds with CUDA 13 3. Disable Numba interop tests because of a regression in Numba-CUDA: see NVIDIA/numba-cuda#611 ### Are these changes tested? Yes, on CI. ### Are there any user-facing changes? No. * GitHub Issue: #47677 Authored-by: Antoine Pitrou <antoine@python.org> Signed-off-by: Raúl Cumplido <raulcumplido@gmail.com>
This reverts commit 9a56516.
This changed the public API of
MemoryPointerand related classes, and the context that they held was used by Arrow (see apache/arrow#48259 (comment)):This commit reverts the change, as it was intended to improve performance without changing functionality, but has had a functional change as a side effect. Following the merge of this PR, we should be able to remove some of the
@require_contextdecorators with some more targeted changes.